-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Don´t print humanized format in single-command mode. #17
base: master
Are you sure you want to change the base?
Conversation
Not caught before because it was not t being tested until this. Confused the heck out of me. Fix by @marcopaganini.
@@ -86,7 +89,7 @@ func stripTrailingDigits(s string, digits int) string { | |||
|
|||
// formatNumber formats the number using base and decimals. For bases different | |||
// than 10, non-integer floating numbers are truncated. | |||
func formatNumber(ctx decimal.Context, n *decimal.Big, base, decimals int) string { | |||
func formatNumber(ctx decimal.Context, n *decimal.Big, base, decimals int, single bool) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"single" doesn't make much sense inside the function. Maybe we should replace it inside the function with "raw", meaning "raw printing" (no formatting) or something similar. In fact, I think we should create two functions, "format" and "formatRaw" that would call this function with raw set to false and true, respectively. Then we could change the rest of the code to call the right function, instead of adding one more mysterious parameter to the call.
@@ -86,7 +89,7 @@ func stripTrailingDigits(s string, digits int) string { | |||
|
|||
// formatNumber formats the number using base and decimals. For bases different | |||
// than 10, non-integer floating numbers are truncated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add the parameter to the comment.
@@ -134,8 +137,8 @@ func formatNumber(ctx decimal.Context, n *decimal.Big, base, decimals int) strin | |||
buf.WriteString(fmt.Sprintf("0x%x%s", n64, suffix)) | |||
default: | |||
h := commafWithDigits(n, decimals) | |||
// Only print humanized format when it differs from original value. | |||
if h != clean { | |||
// Only print humanized format when it differs from original value, and not in single-command mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Try to keep comments formatted to 80 columns.
@@ -200,7 +200,7 @@ func calc(stack *stackType, cmd string) error { | |||
|
|||
if autoprint { | |||
if single { | |||
fmt.Println(stack.top()) // plain print to stdout | |||
fmt.Println(formatNumber(ctx, stack.top(), ops.base, ops.decimals, true)) // plain print to stdout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Avoid inline comments (hard to read, scroll all the way to the right, and no capitalization in this case.)
Instead of doing that:
$ rpn 10 fmt 567 999
wit this fix, it does this:
$ ./rpn 10 fmt 567 999 /
0.5675675676
(ie, no '= ' at the start of the output, and no colors either, so it's ready for use in scripts, etc).